packages/state-watcher/src/cogmesh-poller.ts (2)

46-49: Rapid reload() calls can cause overlapping health checks.

If .retortconfig changes trigger multiple rapid reload() calls (before debouncing in index.ts), several checkHealth() invocations could run concurrently, potentially emitting out-of-order health updates.

Consider tracking an in-flight request to skip or abort previous checks:

🔧 Option: guard against concurrent checks

+  let abortController: AbortController | null = null
+
   async function checkHealth(): Promise<void> {
+    abortController?.abort()
+    abortController = new AbortController()
+
     if (!config.endpoint) {
       onHealth({ status: 'unconfigured' })
       return
     }

     const url = `${config.endpoint.replace(/\/$/, '')}/health`
     const headers: Record<string, string> = {}
     if (config.secret) headers['Authorization'] = `Bearer ${config.secret}`

     const start = Date.now()
     try {
       const res = await fetch(url, {
         headers,
-        signal: AbortSignal.timeout(HEALTH_TIMEOUT_MS),
+        signal: AbortSignal.any([
+          abortController.signal,
+          AbortSignal.timeout(HEALTH_TIMEOUT_MS),
+        ]),
       })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/state-watcher/src/cogmesh-poller.ts` around lines 46 - 49, The
reload() function calls checkHealth() unconditionally which allows overlapping
health checks when reload() is invoked rapidly; modify reload() and
checkHealth() to track an in-flight check (e.g., an inFlight boolean or an
AbortController stored at module scope) so that reload() either cancels the
previous check before starting a new one or skips starting a new check while one
is running; update references to config populated by parseCogmeshConfig(root)
accordingly so the running check can read the correct config snapshot and ensure
checkHealth() respects the cancellation/guard to prevent concurrent executions
and out-of-order health updates.

---

51-54: stop() doesn't cancel in-flight requests.

Calling stop() clears the interval but any ongoing checkHealth() fetch will continue to completion and call onHealth() after the poller is "stopped". This is likely benign but worth noting if shutdown ordering matters.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/state-watcher/src/cogmesh-poller.ts` around lines 51 - 54, The
stop() function currently only clears the interval (timer) but does not cancel
any in-flight checkHealth() fetches, so add cancellation: introduce an
AbortController (or an inFlightController) used by checkHealth() when calling
fetch/HTTP client and store it on the poller; update stop() to call
controller.abort() and set timer = null and a stopped flag; update checkHealth()
to accept/use the AbortSignal and to skip calling onHealth() if the request was
aborted (catch AbortError or check the stopped flag) so no callbacks run after
stop() is called. Ensure you reference and modify stop(), checkHealth(), and
onHealth() accordingly.

packages/state-watcher/src/parsers/retortconfig.ts (2)

33-36: Static analysis flags ReDoS risk — low practical concern given controlled keys.

The key parameter is always hardcoded ('endpoint', 'secret') from within parseCogmeshConfig, so the regex pattern is effectively static. However, if this helper were reused with untrusted keys in the future, the dynamic regex construction could become a vulnerability.

Consider extracting dedicated functions or using a key whitelist if the parser expands.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/state-watcher/src/parsers/retortconfig.ts` around lines 33 - 36, The
extractScalar function builds a dynamic RegExp from the provided key which poses
a ReDoS risk if untrusted keys are ever passed; restrict usage by either only
accepting the known keys used by parseCogmeshConfig ('endpoint' and 'secret') or
replace the dynamic-regex approach with a safe lookup: precompute/compile
separate regexes for those known keys (or whitelist and escape the key before
building the RegExp) and update extractScalar (or introduce
extractEndpoint/extractSecret) accordingly so no arbitrary/unescaped key is
interpolated into a RegExp at runtime.

---

38-41: Unresolved env vars are preserved — verify this is intended.

When ${VAR} references an undefined environment variable, the placeholder is kept verbatim (e.g., the endpoint becomes "https://${MISSING}/api"). This could cause confusing runtime errors from the poller trying to fetch a malformed URL.

Consider logging a warning or returning null when env substitution fails.

🔧 Option: warn on unresolved vars

 function resolveEnv(value: string | null): string | null {
   if (!value) return null
-  return value.replace(/\$\{([^}]+)\}/g, (_, name: string) => process.env[name] ?? `\${${name}}`)
+  return value.replace(/\$\{([^}]+)\}/g, (match, name: string) => {
+    const resolved = process.env[name]
+    if (resolved === undefined) {
+      process.stderr.write(`[retortconfig] Warning: env var ${name} is not set\n`)
+    }
+    return resolved ?? match
+  })
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/state-watcher/src/parsers/retortconfig.ts` around lines 38 - 41, The
resolveEnv function currently preserves unresolved ${VAR} placeholders which can
produce malformed URLs at runtime; update resolveEnv to detect any missing env
names during replacement (e.g., collect names where process.env[name] is
undefined) and if any are missing either log a warning (including the original
input and the missing variable names) and return null, or at minimum return null
when any placeholder remains unresolved; implement this in resolveEnv so callers
(like the poller) get a null instead of a broken string and can handle the error
upstream.

packages/state-watcher/src/parsers/teams.ts (1)

21-29: Potential issue with untrusted marker file content.

If .agentkit-repo contains path separators, leading/trailing slashes, or special characters, the resulting overlay path could resolve unexpectedly (e.g., path traversal or broken paths). Consider sanitizing the marker content.

🛡️ Proposed sanitization

 function detectRepoName(root: string): string {
   try {
     const markerPath = path.join(root, '.agentkit-repo')
     if (fs.existsSync(markerPath)) {
-      return fs.readFileSync(markerPath, 'utf-8').trim()
+      const raw = fs.readFileSync(markerPath, 'utf-8').trim()
+      // Sanitize: strip path separators and use only the basename portion
+      const sanitized = path.basename(raw.replace(/[\\/]/g, '-'))
+      return sanitized || path.basename(root)
     }
   } catch { /* ignore */ }
   return path.basename(root)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/state-watcher/src/parsers/teams.ts` around lines 21 - 29, The
detectRepoName function reads untrusted .agentkit-repo content and returns it
raw which can produce unsafe or unexpected paths; update detectRepoName to
sanitize the marker content read from markerPath: trim it, reject or strip path
separators and any path traversal tokens (e.g., ".."), and restrict characters
(e.g., allow only alphanumerics, dots, underscores, hyphens) or otherwise take
path.basename() of the content; if the sanitized result is empty or invalid,
fall back to path.basename(root) to avoid creating unsafe overlay paths. Ensure
these checks are applied to the value returned by fs.readFileSync before
returning from detectRepoName.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes